wallet+db+waddrmgr: prep for address-manager Store routing#1237
wallet+db+waddrmgr: prep for address-manager Store routing#1237yyforyongyu wants to merge 11 commits into
Conversation
a48b346 to
ace5899
Compare
9a4b083 to
a948f75
Compare
Coverage Report for CI Build 26016618506Warning No base build found for commit Coverage: 81.5%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
ace5899 to
bad6f5a
Compare
6f34422 to
1ae2cdd
Compare
yyforyongyu
left a comment
There was a problem hiding this comment.
LGTM 🚀 (openai/gpt-5.5)
b80a2a4 to
fe1a600
Compare
bad6f5a to
2293674
Compare
fe1a600 to
9980cad
Compare
2293674 to
bb4442b
Compare
3c873d9 to
22e38c9
Compare
bb4442b to
323428d
Compare
22e38c9 to
f2f8777
Compare
323428d to
ffaca1a
Compare
f2f8777 to
46046c0
Compare
d8ffd1f to
1a3c7b4
Compare
|
LGTM 🪶 |
There was a problem hiding this comment.
Pull request overview
Prepares the wallet/db/waddrmgr layers for the upcoming address-manager Store/cache routing work by expanding the address metadata contract, making derived-address creation return/persist pubkeys when available, and standardizing taproot script encoding/decoding for imported taproot script addresses across backends.
Changes:
- Extended
db.AddressInfo/ address row plumbing to include account display metadata,HasScript, andIsUsed(SQL derives used-ness viaEXISTSonutxosper ADR 0011). - Updated derived-address creation flow to allow/persist an optional derived pubkey (
DerivedAddressData.PubKey) and to determine the effective scope address schema from the account row. - Exported waddrmgr taproot TLV codec (
EncodeTaprootScript/DecodeTaprootScript) and a helper to detect presence of private-key material on managed pubkey addresses.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| wallet/internal/sql/sqlite/sqlc/addresses.sql.go | Regenerated sqlc code to project account fingerprint/master key and derived is_used in address reads. |
| wallet/internal/sql/sqlite/queries/addresses.sql | Updated SQLite address read queries to include derived is_used and extra account/wallet fields. |
| wallet/internal/sql/sqlite/migrations/000006_addresses.up.sql | Added schema commentary pointing to ADR rationale for deriving used-ness. |
| wallet/internal/sql/pg/sqlc/addresses.sql.go | Regenerated sqlc code to project derived is_used and account/wallet metadata in address reads. |
| wallet/internal/sql/pg/queries/addresses.sql | Updated Postgres address read queries to include derived is_used and extra account/wallet fields. |
| wallet/internal/sql/pg/migrations/000006_addresses.up.sql | Added schema commentary pointing to ADR rationale for deriving used-ness. |
| wallet/internal/db/sqlite/addresses.go | Threads account metadata into returned AddressInfo; persists derived pubkey; derives IsUsed. |
| wallet/internal/db/sqlite/accounts.go | Adds derived-account address schema extraction from (purpose, coin_type). |
| wallet/internal/db/pg/addresses.go | Threads account metadata into returned AddressInfo; persists derived pubkey; derives IsUsed. |
| wallet/internal/db/pg/accounts.go | Adds derived-account address schema extraction from (purpose, coin_type). |
| wallet/internal/db/interface.go | Extends DerivedAddressData with optional PubKey; clarifies transitional kvdb derive semantics. |
| wallet/internal/db/data_types.go | Extends AddressInfo with account metadata + HasScript and IsUsed. |
| wallet/internal/db/addresses_common.go | Adds account metadata conversion helpers; threads HasScript/IsUsed; updates derived address flow to carry pubkey and schema. |
| wallet/internal/db/addresses_common_test.go | Updates shared helper test for new derived-address inputs/outputs. |
| waddrmgr/tlv.go | Exports taproot script TLV codec; refactors control block record encoding. |
| waddrmgr/address.go | Exports helper to detect private-key material on managed pubkey addresses. |
| docs/developer/adr/README.md | Adds ADR 0011 entry to ADR index. |
| docs/developer/adr/0011-no-addresses-used-column.md | New ADR documenting “derive used-ness from utxos; no used column” decision. |
Files not reviewed (2)
- wallet/internal/sql/pg/sqlc/addresses.sql.go: Language not supported
- wallet/internal/sql/sqlite/sqlc/addresses.sql.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
LGTM 🧱 |
|
LGTM 🪶 ( |
SQL backends derive AddressInfo.IsUsed via EXISTS(utxos) rather than persisting a column. Implementation lands in later commits on this branch.
Expose the taproot script TLV codec so wallet-side import routing can persist encrypted script material through the store seam.
Expose whether a managed public-key address carries encrypted private-key material. The kvdb adapter's address lookup uses this in the watch-only address-info computation: an imported address under a spendable wallet is watch-only at the address row when its underlying `managedAddress` has no encrypted private key. Extracted from the original `kvdb+waddrmgr: add address lookup` commit so the waddrmgr export sits on prep-address-manager-store and the kvdb consumer stays on impl-address-manager-store.
Disambiguates P2TR key-path vs script-path imports — output type alone is ambiguous.
PubKey return on derivedAddressInput + DerivedAddressData. addrSchema parameter so derivedAddressInput honors per-account overrides instead of re-reading scope default. Adds GetAccountAddrSchema adapter field with pg/sqlite scope-default implementations.
Adds AccountNumber, AccountName, KeyScope, MasterKeyFingerprint to AddressInfo and AddressInfoRow. Introduces convertAccountMetadata + convertAddressAccountMetadata + ApplyAddressAccountMetadata so SQL adapters can populate these fields on create + read paths.
SELECT account_master_fingerprint + wallet_master_hd_pub_key from the wallets/accounts JOIN already present in GetAddressByScriptPubKey and ListAddressesByAccount. Regenerates sqlc.
Create + read paths now route through the new helper so AddressInfo returns AccountNumber + AccountName + KeyScope + MasterKeyFingerprint on both backends.
Move the shared SQL derived-address adapter helpers out of the mirrored pg/sqlite closures before wiring IsUsed query projections. The account-schema helper uses the persisted key-scope address type IDs so SQL derived address creation honors stored scope schema overrides.
See ADR 0011 for SQL/kvdb derivation asymmetry.
Wires GetAddressByScriptPubKey + ListAddressesByAccount through EXISTS(SELECT 1 FROM utxos WHERE address_id = ?). The Store interface has no SQL MarkAddressUsed method: the tx-record path already writes the utxos row that the EXISTS derivation reads. Annotates 000006_addresses.up.sql so a future contributor reaching for the column finds ADR 0011.
|
LGTM 🚀 ( |
|
LGTM 🪡 |
|
LGTM 🧪 ( |
Summary
Prep stack that lands the foundation pieces the upcoming address-manager
Store/cache routing PR (#1214) needs, without touching
wallet/address_manager.gorouting or the kvdb address implementation.Base branch:
impl-account-manager-store(PR #1215).Follow-up PR (kvdb impl + manager routing):
impl-address-manager-store(#1214).
What's in scope
waddrmgrexports:ManagedPubKeyAddressHasPrivateKeyso the kvdb address store canderive the spendable-vs-watch-only signal without reaching into private
fields
SQL backends can encode/decode via the same path
dbcontract structural changes:key-scope, master fingerprint, pubkey, script marker, watch-only, and
used-state on
AddressInfoAddressInfoRowplumbing in both pg andsqlite adapters
IsUsedfromEXISTSqueries overutxosrather than persisting anaddresses.usedcolumnnot perform per-address account lookups
transaction, keeping create operations all-or-nothing from the caller's
perspective
of the routing PR's test rewrites
What's NOT in scope
wallet/address_manager.gothroughStoremetadata, secrets) -- these land in the follow-up
impl-address-manager-storePR (wallet: migrate address manager to db.Store #1214)addresses.usedcolumn; ADR 0011 intentionally avoids thatcolumn and uses the
utxostable as the SQL source of truthTest plan
GOWORK=off go test ./wallet ./wallet/internal/db/...GOWORK=off make itest-db db=sqliteGOWORK=off make itest-db db=postgresGOWORK=off make lint-check